fix(influxdb3-ent): widen startup probe window - #827
Conversation
|
Verified locally on a kind cluster using the same node image as CI (v1.31.14), running On the point raised in #800, that probe config cannot affect this much: both readings hold, they just answer different questions. The window does not shorten the replay, and the pod is not Two otherwise identical pods, each taking 90s before opening
For the first one the kubelet logs So a wider window does not make a slow start faster, and this PR does not claim that it does. It stops a slow start from turning into a restart loop. One tradeoff worth stating: a wider window also delays the point at which a pod that will never start gets killed. In CI that is hidden behind the |
|
Updated after a review pass. Four things changed. The window now moves through The arithmetic was one period optimistic. The window is Exit code 137 is Three gaps are documented rather than silently left open. Not addressed here, worth separate issues if wanted: the probe block is shared by all four components, so a misconfigured querier now takes 15 minutes to surface instead of 160s, and |
Nodes replay from object storage on boot, so startup scales with how much data a node reads back; startups past 12 minutes have been reported on clusters with a large history. The 160s window kills them mid-replay, and the restart begins the replay again. Raises failureThreshold alone, as #807 did, leaving the probe cadence untouched. The window is initialDelay + (threshold - 1) x period, so the previous comments were one period optimistic. Documents the probes, which the README did not cover at all: the helm --wait timeout that is shorter than the startup it waits for, the liveness window that takes over once the startup probe succeeds, and the two ways to shorten startup instead of tolerating it. Exit code 137 is also the OOMKilled signature, so troubleshooting says how to tell the two apart.
89caa73 to
ef0e898
Compare
bednar
left a comment
bednar
left a comment
There was a problem hiding this comment.
Two documentation issues need correction before merge; both comments are inline.
…laim The alternatives named mechanisms this chart version does not have: the compacted-data section lands in a separate PR, and ingester.persistence is a deprecated value no template consumes. Exit 137 is one possible signature, not proof. Kubelet asks the runtime to terminate first and honours terminationGracePeriodSeconds, so a process that exits during that window reports a different code, and 137 is also the OOM signature. The termination reason and the probe event are the discriminator.
bednar
left a comment
bednar
left a comment
There was a problem hiding this comment.
Please also update the PR description before merge. It still describes the obsolete configuration as a 930-second window made from a 30-second initial delay and 90 checks every 10 seconds, while this revision keeps a 10-second initial delay and 5-second period and changes failureThreshold from 30 to 184. The description should match the values and timing documented by the final diff. Two additional documentation issues are called out inline.
Kubelet records 'Startup probe failed' for every failed attempt, including ones below failureThreshold, so it does not establish that the probe caused a given restart. The Killing event that reads 'failed startup probe, will be restarted' does. Also softens the either/or, since an externally killed container has other possible causes. The window figure now says it is the point termination is triggered, not when the container has restarted - that follows terminationGracePeriodSeconds.
bednar
left a comment
bednar
left a comment
There was a problem hiding this comment.
Two Kubernetes object-lifecycle terminology corrections are needed; both comments are inline.
| kubectl describe pod -n influxdb3 influxdb3-enterprise-ingester-0 | ||
| ``` | ||
|
|
||
| A pod that restarts during startup while its logs show normal activity is most |
There was a problem hiding this comment.
A Pod itself does not restart when a startup probe fails; kubelet restarts the affected container within the existing Pod. Please change this to A container that restarts during startup... or A pod whose application container restarts during startup... so the troubleshooting guidance describes the lifecycle accurately.
| | `probes.startup.initialDelaySeconds` | Delay before the first startup check | `10` | | ||
| | `probes.startup.periodSeconds` | Interval between startup checks | `5` | | ||
| | `probes.startup.timeoutSeconds` | Timeout of a single startup check | `5` | | ||
| | `probes.startup.failureThreshold` | Failed startup checks before the pod is killed | `184` | |
There was a problem hiding this comment.
The startup probe does not kill the Pod. After failureThreshold consecutive failures, kubelet triggers termination and restart of the affected container while the Pod remains. Please describe this as, for example, Failed startup checks before container termination is triggered; that also remains consistent with the terminationGracePeriodSeconds explanation above.
Widens the startup probe window and documents the probe settings, which the README did not cover at all.
Why
Nodes replay from object storage on boot, so startup scales with how much data a node reads back. Support cases report startups past 12 minutes, while the window was 155s in practice. A node killed mid-replay restarts and begins the replay again.
What changes
failureThresholdgoes from 30 to 184, leavinginitialDelaySeconds: 10andperiodSeconds: 5untouched, as #807 did when it last raised the window. That gives10 + (184 - 1) × 5 = 925sbefore the probe triggers termination; the restart itself then followsterminationGracePeriodSeconds.The arithmetic in the old comments was one period optimistic - the window is
initialDelay + (threshold - 1) × period, so the documented10s + (30 × 5s) = 160swas really 155s. Comments and docs now use the correct form.Documentation
A Health Probes section covering three things that were not written down anywhere:
helm --waitdefaults to a five-minute timeout, shorter than the startup it waits for; the liveness probe keeps a narrow3 × 10swindow once the startup probe succeeds; and the troubleshooting note now uses theKillingevent as the discriminator rather than exit code 137, which is also the OOMKilled signature.Testing
helm lint, a render of all four CI values files and stockvalues.yaml, and a kind cluster on the CI node image running the fullct lintandct installchain. The probe mechanism itself was verified separately: two otherwise identical pods taking 90s to open/health, one with a window shorter than the startup and one longer - the first is killed and restarts, the second reachesReady.Supersedes #801, which proposed 230s before the longer startups were reported.
Closes #800